Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(connectivity_plus): Return multiple active types on MacOS/iOS when available #3083

Closed
wants to merge 1 commit into from

Conversation

vbuberen
Copy link
Collaborator

@vbuberen vbuberen commented Jul 9, 2024

Description

Switching from check for active used connection on iOS/MacOS to available connections to return multiple connection types when such are available. Tested with a dongle for MacBook which has Ethernet port and looks like getting results as expected:
Screenshot 2024-07-09 at 18 08 41
Here is a screenshot from a few more test cases where I tried to connect/disconnect both Wi-Fi and Ethernet
Screenshot 2024-07-09 at 18 13 41

Additionally did a few changes to the API documentation comments.

Related Issues

Closes #3073

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

@vbuberen vbuberen added the connectivity_plus Connectivity Plus label Jul 9, 2024
@vbuberen vbuberen changed the title feat(connectivity_plus): Return multiple active types on MacOS when available feat(connectivity_plus): Return multiple active types on MacOS/iOS when available Jul 9, 2024
@vbuberen
Copy link
Collaborator Author

vbuberen commented Jul 9, 2024

Would appreciate an additional check on how it works on iOS with mobile and wi-fi active as don't have iPhone with a SIM card

@miquelbeltran
Copy link
Member

Would appreciate an additional check on how it works on iOS with mobile and wi-fi active as don't have iPhone with a SIM card

I should be able to do that.

Btw, I think we can change the Android implementation to also report the available connections instead of just the used ones, but I don't remember exactly now what was the api.

@miquelbeltran
Copy link
Member

Maybe we should implement this change in a different stream listener?

In other words:

  • checkConnectivity and onConnectivityChanged continue to report the "active" used connection
  • A new property availableConnectivity and onAvailableConnectivityChanged report on all available interfaces, even when not actively used.

I know it is more work, but I think there is still a valid use case for the previous implementation.

While we implement the rest of the platforms, we can mirror availableConnectivity to checkConnectivity if no specific implementation for availableConnectivity exists.

@vbuberen
Copy link
Collaborator Author

Btw, I think we can change the Android implementation to also report the available connections instead of just the used ones, but I don't remember exactly now what was the api.

For some reason I thought that on Android we already do it this way and return multiple types after introducing this feature with multiple types returned. But seems like I confused something as tested and see that no, we return only the one currently active/used ones.

In other words:
checkConnectivity and onConnectivityChanged continue to report the "active" used connection
A new property availableConnectivity and onAvailableConnectivityChanged report on all available interfaces, even when not actively used.

I like this idea. It would make sense for users, who rely on onConnectivityChanged as a way to determine if user has internet (even though the plugin doesn't guarantee that, but still most people use the plugin for this exact purpose).
Will need to do some additional research on APIs to achieve this.

@miquelbeltran
Copy link
Member

miquelbeltran commented Jul 14, 2024

If I remember correctly, the hasTransport() method says if the interface exists, and hasCapability() if it is being used. Both are on the NetworkCapabilities class, so the implementation shouldn't be too different.

@vbuberen
Copy link
Collaborator Author

If I remember correctly, the hasTransport() method says if the interface exists, and hasCapability() if it is being used. Both are on the NetworkCapabilities class, so the implementation shouldn't be too different.

Yeah, that is for Android. I meant more Windows/Linux platforms as a subject for research as not really familiar with them.

@vbuberen
Copy link
Collaborator Author

I don't see any activity in the related issue, so will just close the PR, considering having no real value in expanding API that way.

@vbuberen vbuberen closed this Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants